New version.

This seems a safer than 0 and is infinitely better for offset arrays! :)

This was said in another forum but I’ll say it one time more - this seems like the kind of condition that can only be dealt with properly when the caller explicitly checks the result. To me this is a textbook case of using something like Nullable or the much discussed Some. Using some wrapped (and then explicitly unrwapped, by the caller) return type would allow the API to extend to other containers like AbstractDicts which allow anything as keys.

Just thoughts - don’t let this slow progress on AbstractArray.

@andyferris Better continue the discussion about Some at JuliaLang/Juleps#47. The problem with that solution (which is indeed more correct in general) is that it would be quite inconvenient when working with arrays, which are a major use case for find*.

@nalimilan On one hand, I completely agree, let's just move forward with a simple thing which is correct for arrays (including offset arrays).

The problem with that solution (which is indeed more correct in general) is that it would be quite inconvenient when working with arrays

On the other hand, the underlying thing which bothers me here and in JuliaLang/Juleps#47 is that we discuss this "inconvenience" in a strange way - even for arrays, it is absolutely necessary for the caller to do something to deal with the fact that the index might not be found. Whichever way we disguise it (with sentinel values, union types, wrapping types, etc), the caller really needs to check some boolean condition whether a matching index was found or not. My judgement is that wrapping the return in a Nullable or Some is the best way to make the caller explicitly aware that this simply has to happen (or else an error is thrown). The union approach might mean that a suite of unit tests could pass if the author doesn't remember to cover the general case you might see with unknown, production data where no index is found.

But yes, perhaps this is something better discussed elsewhere! Like I tried to say earlier, I only want to write this for the record on JuliaLang/julia for future reference (say, for v2.0 development), rather than do anything to block this PR (which is clearly better than the status quo, and therefore quite acceptable).

OK, barring more objections I think this is ready. Let's first get a performance test:

@nanosoldier runbenchmarks(ALL, vs=":master")

Thanks Tim! BTW, I think you missed my question at #24774 (comment), which is now hidden.

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo cset shield -e su nanosoldier -- -c ./benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here
cc @ararslan

Nanosoldier is hitting a MethodError related to the new return type from find. Perhaps something that will need to be addressed in BaseBenchmarks before Nanosoldier will work here.

Unfortunately, it's going to be difficult to support both the new and the old API. I guess we'll need to add a Compat.find function returning nothing even on 0.6.

I see slight regressions in

julia> @btime perf_findnext($x)    # length-1000 Bool vector
  1.234 μs (0 allocations: 0 bytes)

vs

julia> @btime perf_findnext($x)
  837.929 ns (0 allocations: 0 bytes)

There are a couple of apparent big regressions in what appears to be completely unrelated code (e.g., a ccall inside of resize!), I'm pretty puzzled about that, but it also doesn't seem to be very reproducible.

I'm not seeing any complaints about that regression. Any objections to merging? If not, I'll fix the conflicts.

I just noticed that indexin still uses 0 to denote missing values. I previously proposed some indexin improvements in #23845, but I am not sure how to handle this. Note that the function, with or without #23845, fails when an array with custom indices is passed as the second argument. A (relatively extensive) list of options I see:

1. Leave it as is (perhaps eventually incorporating Make indexin first argument accept any iterable #23845). But then there is pretty serious inconsistency in the API, as I know of no other function that still uses 0 to denote missing data.
2. Use nothing as the sentinel for indexin, which would allow expanding its usage to arrays with custom indices. But then the most common case of returning an array means that each element is boxed IIUC, as the data type of an element is Union{Nothing,Int}.
3. Error on missing data. This will be most efficient in the use cases where missing data is not anticipated, and will also allow using arrays with custom indices. But it is a break from current behavior.
4. Some combination of the above two options, based on a keyword argument (or a similar mechanism, if that were to break inference).
5. Remove indexin from Base altogether. It is not used in Base, and it seems to be used somewhat rarely throughout the package ecosystem (complete list of uses here). Given that there are multiple behaviors one might expect for missing data, perhaps it is better for each individual user to write this few-line function using the error handling that they expect. The (only) downside (that I can think of) is that this removes an interface that might be nice for some data structures, which would be missed by people rolling their own implementations. For instance, UniqueVectors implements this method in a way that skips the step of building the lookup dictionary (since it already has access to such a Dict). But this seems a relatively specialized use case, not a compelling case for keeping this function in Base.

My overall vote (at the moment) is the last option: removing indexin from Base. I think a decent case could also be made for the third or fourth options, but even so I doubt the function actually belongs in Base, rather it should be in a (standard?) library somewhere.

I had noticed that too, and I think we should return nothing instead of 0 for consistency with other functions. Union{Nothing,Int} arrays use an efficient memory representation on 0.7 already, and since we return nothing with all other find* functions, we already assume performance is OK (or can be fixed).

This needs a NEWS entry

#26067